-
Notifications
You must be signed in to change notification settings - Fork 244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(avm): operands reordering #10182
Conversation
Ok(()) | ||
} | ||
} | ||
|
||
impl AvmInstruction { | ||
/// Bytes representation for generating AVM bytecode | ||
/// Order: INDIRECT, OPERANDS, TAG, IMMEDIATES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, in principle I saw no reason to introduce the immediates separately, since you could've just modified the particular opcodes and put the immediate at the end. However then I saw you have the tag in between them. Is this needed? if you do: Indirect, addresses, immediates, tag
then I think you wouldn't need to separate the immediates and that seems conceptually right?
Or do you need the tag to be in the middle for some reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fcarreiro I had the same thoughts and the reason I nevertheless introduced the immediates is for the SET opcode. If we put the tag before the immediates then all set opcode flavors start the same which would help to have less complex relations in bytecode decomposition.
I thought that conceptually to have "immediates" is not too bad neither.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, the offset for TAG in set opcode would be at the same position (except SET_8) for all flavors. If we put TAG at the end, then we need to offset by the different immediate sizes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Ok, not too bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that you don't need to make these ones match the serialization. In TS you have to, but here you are ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I have found it convenient for reading purposes but did not perform this for the SET opcode as it would have been too tedious.
Ok(()) | ||
} | ||
} | ||
|
||
impl AvmInstruction { | ||
/// Bytes representation for generating AVM bytecode | ||
/// Order: INDIRECT, OPERANDS, TAG, IMMEDIATES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Ok, not too bad.
barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp
Outdated
Show resolved
Hide resolved
* master: (106 commits) feat: blobs. (#9302) chore(avm): operands reordering (#10182) feat: UltraRollupRecursiveFlavor (#10088) feat: one liner for nodes to join rough-rhino (#10168) feat!: rename sharedimmutable methods (#10164) chore(master): Release 0.64.0 (#10043) feat: e2e metrics reporting (#9776) chore: fix pool metrics (#9652) chore: Initial draft of testnet-runbook (#10085) feat: Improved data storage metrics (#10020) chore: Remove handling of duplicates from the note hash tree (#10016) fix: add curl to aztec nargo container (#10173) fix: Revert "feat: integrate base fee computation into rollup" (#10166) feat!: rename SharedMutable methods (#10165) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg feat: sync tags as sender (#10071) feat: integrate base fee computation into rollup (#10076) ...
* master: (64 commits) fix: docker compose aztec up fix (#10197) fix: aztec-nargo curl in the earthfile also (#10199) chore: fix devbox (#10201) chore: misc cleanup (#10194) fix: release l1-contracts (#10095) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg feat: Origin tags implemented in biggroup (#10002) fix: Revert "feat: blobs. (#9302)" (#10187) feat!: remove SharedImmutable (#10183) fix(bb.js): don't minify bb.js - webpack config (#10170) feat: blobs. (#9302) chore(avm): operands reordering (#10182) feat: UltraRollupRecursiveFlavor (#10088) feat: one liner for nodes to join rough-rhino (#10168) feat!: rename sharedimmutable methods (#10164) chore(master): Release 0.64.0 (#10043) feat: e2e metrics reporting (#9776) ...
🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.65.0</summary> ## [0.65.0](aztec-package-v0.64.0...aztec-package-v0.65.0) (2024-11-26) ### Features * **avm:** New public inputs witgen ([#10179](#10179)) ([ac8f13e](ac8f13e)) </details> <details><summary>barretenberg.js: 0.65.0</summary> ## [0.65.0](barretenberg.js-v0.64.0...barretenberg.js-v0.65.0) (2024-11-26) ### Bug Fixes * **bb.js:** Don't minify bb.js - webpack config ([#10170](#10170)) ([6e7fae7](6e7fae7)) </details> <details><summary>aztec-packages: 0.65.0</summary> ## [0.65.0](aztec-packages-v0.64.0...aztec-packages-v0.65.0) (2024-11-26) ### ⚠ BREAKING CHANGES * remove SharedImmutable ([#10183](#10183)) * rename sharedimmutable methods ([#10164](#10164)) ### Features * **avm:** New public inputs witgen ([#10179](#10179)) ([ac8f13e](ac8f13e)) * Blobs. ([#9302](#9302)) ([03b7e0e](03b7e0e)) * One liner for nodes to join rough-rhino ([#10168](#10168)) ([3a425e9](3a425e9)) * Origin tags implemented in biggroup ([#10002](#10002)) ([c8696b1](c8696b1)) * Remove SharedImmutable ([#10183](#10183)) ([a9f3b5f](a9f3b5f)) * Rename sharedimmutable methods ([#10164](#10164)) ([ef7cd86](ef7cd86)) * UltraRollupRecursiveFlavor ([#10088](#10088)) ([4418ef2](4418ef2)) ### Bug Fixes * Aztec-nargo curl in the earthfile also ([#10199](#10199)) ([985a678](985a678)) * **bb.js:** Don't minify bb.js - webpack config ([#10170](#10170)) ([6e7fae7](6e7fae7)) * Docker compose aztec up fix ([#10197](#10197)) ([d7ae959](d7ae959)) * Increase test timeouts ([#10205](#10205)) ([195aa3d](195aa3d)) * Release l1-contracts ([#10095](#10095)) ([29f0d7a](29f0d7a)) * Revert "feat: blobs. ([#9302](#9302))" ([#10187](#10187)) ([a415f65](a415f65)) ### Miscellaneous * Added ref to env variables ([#10193](#10193)) ([b51fc43](b51fc43)) * **avm:** Operands reordering ([#10182](#10182)) ([69bdf4f](69bdf4f)), closes [#10136](#10136) * Fix devbox ([#10201](#10201)) ([323eaee](323eaee)) * Misc cleanup ([#10194](#10194)) ([dd01417](dd01417)) * Reinstate docs-preview, fix doc publish ([#10213](#10213)) ([ed9a0e3](ed9a0e3)) * Replace relative paths to noir-protocol-circuits ([1650446](1650446)) </details> <details><summary>barretenberg: 0.65.0</summary> ## [0.65.0](barretenberg-v0.64.0...barretenberg-v0.65.0) (2024-11-26) ### Features * **avm:** New public inputs witgen ([#10179](#10179)) ([ac8f13e](ac8f13e)) * Origin tags implemented in biggroup ([#10002](#10002)) ([c8696b1](c8696b1)) * UltraRollupRecursiveFlavor ([#10088](#10088)) ([4418ef2](4418ef2)) ### Miscellaneous * **avm:** Operands reordering ([#10182](#10182)) ([69bdf4f](69bdf4f)), closes [#10136](#10136) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.65.0</summary> ## [0.65.0](AztecProtocol/aztec-packages@aztec-package-v0.64.0...aztec-package-v0.65.0) (2024-11-26) ### Features * **avm:** New public inputs witgen ([#10179](AztecProtocol/aztec-packages#10179)) ([ac8f13e](AztecProtocol/aztec-packages@ac8f13e)) </details> <details><summary>barretenberg.js: 0.65.0</summary> ## [0.65.0](AztecProtocol/aztec-packages@barretenberg.js-v0.64.0...barretenberg.js-v0.65.0) (2024-11-26) ### Bug Fixes * **bb.js:** Don't minify bb.js - webpack config ([#10170](AztecProtocol/aztec-packages#10170)) ([6e7fae7](AztecProtocol/aztec-packages@6e7fae7)) </details> <details><summary>aztec-packages: 0.65.0</summary> ## [0.65.0](AztecProtocol/aztec-packages@aztec-packages-v0.64.0...aztec-packages-v0.65.0) (2024-11-26) ### ⚠ BREAKING CHANGES * remove SharedImmutable ([#10183](AztecProtocol/aztec-packages#10183)) * rename sharedimmutable methods ([#10164](AztecProtocol/aztec-packages#10164)) ### Features * **avm:** New public inputs witgen ([#10179](AztecProtocol/aztec-packages#10179)) ([ac8f13e](AztecProtocol/aztec-packages@ac8f13e)) * Blobs. ([#9302](AztecProtocol/aztec-packages#9302)) ([03b7e0e](AztecProtocol/aztec-packages@03b7e0e)) * One liner for nodes to join rough-rhino ([#10168](AztecProtocol/aztec-packages#10168)) ([3a425e9](AztecProtocol/aztec-packages@3a425e9)) * Origin tags implemented in biggroup ([#10002](AztecProtocol/aztec-packages#10002)) ([c8696b1](AztecProtocol/aztec-packages@c8696b1)) * Remove SharedImmutable ([#10183](AztecProtocol/aztec-packages#10183)) ([a9f3b5f](AztecProtocol/aztec-packages@a9f3b5f)) * Rename sharedimmutable methods ([#10164](AztecProtocol/aztec-packages#10164)) ([ef7cd86](AztecProtocol/aztec-packages@ef7cd86)) * UltraRollupRecursiveFlavor ([#10088](AztecProtocol/aztec-packages#10088)) ([4418ef2](AztecProtocol/aztec-packages@4418ef2)) ### Bug Fixes * Aztec-nargo curl in the earthfile also ([#10199](AztecProtocol/aztec-packages#10199)) ([985a678](AztecProtocol/aztec-packages@985a678)) * **bb.js:** Don't minify bb.js - webpack config ([#10170](AztecProtocol/aztec-packages#10170)) ([6e7fae7](AztecProtocol/aztec-packages@6e7fae7)) * Docker compose aztec up fix ([#10197](AztecProtocol/aztec-packages#10197)) ([d7ae959](AztecProtocol/aztec-packages@d7ae959)) * Increase test timeouts ([#10205](AztecProtocol/aztec-packages#10205)) ([195aa3d](AztecProtocol/aztec-packages@195aa3d)) * Release l1-contracts ([#10095](AztecProtocol/aztec-packages#10095)) ([29f0d7a](AztecProtocol/aztec-packages@29f0d7a)) * Revert "feat: blobs. ([#9302](AztecProtocol/aztec-packages#9302))" ([#10187](AztecProtocol/aztec-packages#10187)) ([a415f65](AztecProtocol/aztec-packages@a415f65)) ### Miscellaneous * Added ref to env variables ([#10193](AztecProtocol/aztec-packages#10193)) ([b51fc43](AztecProtocol/aztec-packages@b51fc43)) * **avm:** Operands reordering ([#10182](AztecProtocol/aztec-packages#10182)) ([69bdf4f](AztecProtocol/aztec-packages@69bdf4f)), closes [#10136](AztecProtocol/aztec-packages#10136) * Fix devbox ([#10201](AztecProtocol/aztec-packages#10201)) ([323eaee](AztecProtocol/aztec-packages@323eaee)) * Misc cleanup ([#10194](AztecProtocol/aztec-packages#10194)) ([dd01417](AztecProtocol/aztec-packages@dd01417)) * Reinstate docs-preview, fix doc publish ([#10213](AztecProtocol/aztec-packages#10213)) ([ed9a0e3](AztecProtocol/aztec-packages@ed9a0e3)) * Replace relative paths to noir-protocol-circuits ([1650446](AztecProtocol/aztec-packages@1650446)) </details> <details><summary>barretenberg: 0.65.0</summary> ## [0.65.0](AztecProtocol/aztec-packages@barretenberg-v0.64.0...barretenberg-v0.65.0) (2024-11-26) ### Features * **avm:** New public inputs witgen ([#10179](AztecProtocol/aztec-packages#10179)) ([ac8f13e](AztecProtocol/aztec-packages@ac8f13e)) * Origin tags implemented in biggroup ([#10002](AztecProtocol/aztec-packages#10002)) ([c8696b1](AztecProtocol/aztec-packages@c8696b1)) * UltraRollupRecursiveFlavor ([#10088](AztecProtocol/aztec-packages#10088)) ([4418ef2](AztecProtocol/aztec-packages@4418ef2)) ### Miscellaneous * **avm:** Operands reordering ([#10182](AztecProtocol/aztec-packages#10182)) ([69bdf4f](AztecProtocol/aztec-packages@69bdf4f)), closes [#10136](AztecProtocol/aztec-packages#10136) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Resolves #10136